commit finder: improve version commit finder for codediff tool #504
Conversation
academo
commented
Feb 5, 2026
academo
commented
- Fetch remote tags on shallow clone to be able to switch refs
- Fetch remote ref to be able to switch branches and tags
- Fallback into gemini-cli for commit finding
| @@ -0,0 +1,53 @@ | |||
| // Package main provides a manual test harness for versioncommitfinder. | |||
There was a problem hiding this comment.
this file exist to test the gemini prompt locally. I am leaving it here for future development.
the file itself is never imported or built in the validator.
There was a problem hiding this comment.
This file looks quite easy to regenerate. Do you really need it? IMO it would polute the repo
There was a problem hiding this comment.
that's a fair point. I'll remove it.
There was a problem hiding this comment.
I think you accidentally re-added it
There was a problem hiding this comment.
yes! I was testing the recent changes. I'll remove it before I ask for another round of reviews
| ctx, | ||
| "gemini", | ||
| "-m", | ||
| "gemini-2.5-flash", |
There was a problem hiding this comment.
There was a problem hiding this comment.
gemini 3 flash performed worst in my tests over 2.5 and it was much slower. that's why I kept 2.5 flash. we should keep an eye when it becomes deprecated.
There was a problem hiding this comment.
maybe add a comment explaining that?
academo
changed the title
| @@ -0,0 +1,53 @@ | |||
| // Package main provides a manual test harness for versioncommitfinder. | |||
There was a problem hiding this comment.
This file looks quite easy to regenerate. Do you really need it? IMO it would polute the repo
| ctx, | ||
| "gemini", | ||
| "-m", | ||
| "gemini-2.5-flash", |
There was a problem hiding this comment.
maybe add a comment explaining that?
andresmgot
left a comment
andresmgot
left a comment
There was a problem hiding this comment.
LGTM, a couple of nits
| client := llmclient.NewGeminiClient() | ||
| if err := client.CanUseLLM(); err != nil { | ||
| return "", fmt.Errorf("cannot use LLM: %w", err) | ||
| } |
There was a problem hiding this comment.
nit: Usually you do this kind of checks before calling the function
github-project-automation
bot
moved this from 🔬 In review
to 🚀 Shipped
in Grafana Catalog Team
3 participants
